Skip to content

Conversation

@veerababu1729
Copy link

Description

Fixes Windows-specific EINVAL error when spawning npm install commands.

Problem

On Windows, the run_npm_install function in Kernel.js was failing with spawn EINVAL error because it was missing the shell: true option when spawning npm.cmd.

Solution

  • Added shell: true to spawn options in run_npm_install function
  • Makes the spawn call consistent with other spawn calls in the codebase (ProcessService.js and LocalTerminalService.js)
  • Cross-platform compatible (works on Windows, Linux, macOS)

Changes

  • File: src/backend/src/Kernel.js
  • Line: 581
  • Change: Added shell: true to spawn options

Fixes

Why Windows Needs shell: true:

  • On Windows, npm.cmd is a batch file, not an executable
  • Node.js spawn() cannot execute .cmd files directly without shell: true
  • Without it, Windows throws EINVAL (Invalid argument) error

Proof from Codebase:
Other spawn calls in the same codebase already use shell: true:

  • ProcessService.js line 76: spawn(command, args, { shell: true, ... })
  • LocalTerminalService.js line 93: spawn(profile.shell[0], args, { shell: true, ... })

Testing

  • Verified fix follows existing code patterns in the codebase
  • Minimal change with low risk
  • No functional changes to other parts of the system

Type of Change

  • Bug fix (non-breaking change which fixes an issue)

- Fixes EINVAL error when running npm install on Windows
- Adds shell: true to spawn options in run_npm_install function
- Makes spawn call consistent with other spawn calls in codebase
- Resolves GitHub issues HeyPuter#1748 and HeyPuter#1797
@CLAassistant
Copy link

CLAassistant commented Oct 23, 2025

CLA assistant check
All committers have signed the CLA.

@veerababu1729
Copy link
Author

I almost worked one day to find this, please review as soon as possible and feel free to ask anything...

@jelveh
Copy link
Contributor

jelveh commented Oct 26, 2025

Hey @veerababu1729. thank you for you PR. I've assigned @ProgrammerIn-wonderland to review this. Please stay tuned.

@ProgrammerIn-wonderland
Copy link
Collaborator

I think @KernelDeimos mentioned on call that we shouldn't do (shell: true) since it says a deprecation warning in shell but I'll check if that's still the case

@ProgrammerIn-wonderland
Copy link
Collaborator

yeah this gives me the deprecation warning on startup. Instead of using shell: true, maybe we should stop using npm.cmd on windows? regular npm works fine for me

@KernelDeimos
Copy link
Contributor

I suspect shell: true is necessary in some cases. The solution might be to try everything and see what sticks.

Yesterday I wrote a good explanation of the situation but GitHub didn't save the comment I apparently didn't send so we can't have everything; I'll try my best to recreate that: apparently the Node.js maintainers decided that passing args with shell:true should be deprecated which means it might be removed in a future version of node (i.e. instead of getting a warning here, this would be an error and Puter won't be able to continue running). The rationale here makes sense: the fact that spawn accepts this array of input args kind of implies that it will properly process and escape them for you, but it doesn't. (and in hindsight, how could it with shell: true? There are so many shells!)

In our case the security concern isn't applicable - we're passing a static string. It turns out (and I didn't realize this) that you can just pass the argument in the string in the first parameter. This is only available with shell: true so the difference here is really executing a process given its path vs executing a "whatever this string does in the shell" using the shell.

Here's my "try everything and see what sticks" solution:

const executables = ['npm', 'npm.cmd'];
const spawners = [
  (exe) => spawn(exe, 'install', { cwd: path, stdio: "pipe" }),
  (exe) => spawn(`${exe} install`, { cwd: path, stdio: "pipe" }),
];

for ( const exe of executables ) {
  for ( const spawner of spawners ) {
    try {
      spawner(exe);
    } catch (e) {
      // ...
      continue;
    }
    break;
  }
}
const proc = spawn(npmCmd + ' install', { shell: true, cwd: path, stdio: "pipe" });

@veerababu1729
Copy link
Author

Let me test your proposal code and let you know.......

@ProgrammerIn-wonderland
Copy link
Collaborator

any updates

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Invalid Host header. & workers error Windows Error

5 participants